imageCaptureProcessing maintain EXIF #2902

melmathari · 2024-11-24T18:15:56Z

Images taken in CommCare were losing their EXIF metadata, including critical GPS/location data, during processing. This created headaches for users who needed that juicy metadata intact, especially for location-based verification.

Resolves issue #2689 and #2743

The Fix

We pimped the image capture and processing workflow to make EXIF data stick, no matter what. Here's what we did:

Injected EXIF-aware copying for raw image files.
Ensured EXIF data is kept intact during image scaling.
Made sure key EXIF tags stay in the game:
- GPS coordinates (latitude/longitude)
- GPS timestamps
- Image datetime
- Orientation metadata

How We Pulled It Off

Created copyFileWithExifData() to keep EXIF data alive during file copying.
Introduced scaleAndSaveImageWithExif() for maintaining EXIF while scaling images.
Modified ImageCaptureProcessing to use our new EXIF-hugging methods.
Leveraged Android's ExifInterface API for bulletproof metadata handling.

Test Drive

Basic EXIF Check
- Snap a pic with GPS turned on.
- Save the form.
- Check that all EXIF data is intact using your favorite EXIF viewer.
Scaled Image Test
- Take a high-res image.
- Verify the EXIF data after it gets scaled down.
Encrypted Image Test
- Enable media encryption in settings.
- Capture a GPS-tagged image.
- Confirm EXIF data survived the encryption.
Edge Cases
- Use images with no EXIF data.
- Test images from various sources (camera, gallery, etc.).
- Try different formats (JPG, PNG).

Tools to Verify

Command line nerds: Use exiftool to peek at the EXIF data.

Additional Notes

Full backward compatibility with existing image operations.
No database or form schema changes needed.
Minimal performance impact, as EXIF operations run only on image files.

/claim #2689

damagatchi · 2024-11-24T18:15:58Z

Can one of the admins verify this patch?

algora-pbc · 2024-11-24T18:32:19Z

Thank you for the submission!

Please ensure you meet the eligibility criteria for our program.

📝 Our team will review and process the payment if everything looks good.

shubham1g5

Hey @melmathari. Your attempts looks great. Made some clarifying comments, but also wondering if it's possible to add unit tests for the new FileUtil methods you have added to copy the exif data. Thanks!

app/src/org/commcare/utils/FileUtil.java

shubham1g5 · 2024-11-25T06:38:25Z

app/src/org/commcare/utils/FileUtil.java

+        ExifInterface destExif = new ExifInterface(destFile.getAbsolutePath());
+
+        // Copy GPS data
+        String[] tagsToPreserve = {


Think we should also include these tags here -

TAG_COPYRIGHT,
TAG_IMAGE_DESCRIPTION,
TAG_DATETIME_DIGITIZED,
TAG_DATETIME_ORIGINAL,
TAG_EXIF_VERSION,
TAG_OFFSET_TIME
,TAG_OFFSET_TIME_ORIGINAL
,TAG_OFFSET_TIME_DIGITIZED,
TAG_GPS_ALTITUDE,
TAG_GPS_ALTITUDE_REF,
TAG_GPS_AREA_INFORMATION

shubham1g5 · 2024-11-25T06:40:51Z

app/src/org/commcare/utils/FileUtil.java

+            ExifInterface.TAG_GPS_LATITUDE_REF,
+            ExifInterface.TAG_GPS_LONGITUDE,
+            ExifInterface.TAG_GPS_LONGITUDE_REF,
+            ExifInterface.TAG_GPS_TIMESTAMP,
+            ExifInterface.TAG_GPS_DATESTAMP,
+            // Preserve other important EXIF data
+            ExifInterface.TAG_DATETIME,
+            ExifInterface.TAG_ORIENTATION
+        };
+
+        for (String tag : tagsToPreserve) {
+            String value = sourceExif.getAttribute(tag);
+            if (value != null) {
+                destExif.setAttribute(tag, value);
+            }
+        }
+


Is there a reason we are not directly using the method copyExifData here ?

shubham1g5 · 2024-11-25T08:58:41Z

app/src/org/commcare/utils/FileUtil.java

+        return scaled;
+    }
+
+    private static void copyExifData(ExifInterface source, ExifInterface dest) {


how about directly passing source and destination file paths to this method ?

shubham1g5 · 2024-11-25T09:00:46Z

@damagatchi ok to test

melmathari · 2024-11-25T11:03:33Z

@shubham1g5 I've addressed the feedback. As for the unit tests, I'm not sure if I'll be able to complete them, i ran into several issues with testImplementation project(path: ':commcare-core', configuration: 'testsAsJar')

shubham1g5

@melmathari It's fine to ignore the lint errors in the code you have not changed as the method refactoring will need to be replicated elsewhere in the calling code.

shubham1g5 · 2024-11-25T11:11:01Z

app/src/org/commcare/utils/FileUtil.java

@@ -121,7 +121,7 @@ public static boolean cleanFilePath(String fullPath, String extendedPath) {

    private static final String illegalChars = "'*','+'~|<> !?:./\\";

-    public static String SanitizeFileName(String input) {
+    public static String sanitizeFileName(String input) {


think you can ignore lint errors in the code you have not changed as method renames will need to be replicated while calling the method as well.

Sorry if I wasn't clear but the file name will need to be reverted here, as otherwise the project fails to compile.

shubham1g5 · 2024-11-25T11:13:42Z

app/src/org/commcare/utils/FileUtil.java

@@ -356,18 +356,17 @@ public static String getGlobalStringUri(String fileLocation) {
        return "file://" + fileLocation;
    }

-    public static void checkReferenceURI(Resource r, String URI, Vector<MissingMediaException> problems) throws InvalidReferenceException {
-        Reference mRef = ReferenceManager.instance().DeriveReference(URI);
+    public static Reference checkReferenceUri(String uri) throws InvalidReferenceException {


this is causing the build to fail now due to method calls not having the corresponding change, you can ignore lint errors here.

this will need to be reverted.

shubham1g5 · 2024-11-25T11:25:59Z

As for the unit tests, I'm not sure if I'll be able to complete them, i ran into several issues with testImplementation project(path: ':commcare-core', configuration: 'testsAsJar')

Curious what kind of errors are you facing here and if you have followed the instructions given here to run tests.

melmathari · 2024-11-25T11:34:53Z

@shubham1g5 Thanks for providing the doc. Unfortunately, I won't be able to commit to the unit testing for this build due to my current schedule. We have tested this on our local implementation though.

shubham1g5 · 2024-11-25T11:37:37Z

app/lint.xml

+        <ignore path="src/org/commcare/utils/FileUtil.java" />
+        <ignore path="src/org/commcare/activities/components/ImageCaptureProcessing.java" />


Sorry, I didn't mean to turn off lint checks for these files, but just to ignore any errors that you have not changed (lint errors doesn't block the merge/approval on PR for us) .

shubham1g5 · 2024-11-25T11:38:33Z

app/src/org/commcare/utils/FileUtil.java

@@ -121,7 +121,7 @@ public static boolean cleanFilePath(String fullPath, String extendedPath) {

    private static final String illegalChars = "'*','+'~|<> !?:./\\";

-    public static String SanitizeFileName(String input) {
+    public static String sanitizeFileName(String input) {


Sorry if I wasn't clear but the file name will need to be reverted here, as otherwise the project fails to compile.

shubham1g5 · 2024-11-25T11:38:47Z

app/src/org/commcare/utils/FileUtil.java

@@ -356,18 +356,17 @@ public static String getGlobalStringUri(String fileLocation) {
        return "file://" + fileLocation;
    }

-    public static void checkReferenceURI(Resource r, String URI, Vector<MissingMediaException> problems) throws InvalidReferenceException {
-        Reference mRef = ReferenceManager.instance().DeriveReference(URI);
+    public static Reference checkReferenceUri(String uri) throws InvalidReferenceException {


this will need to be reverted.

melmathari · 2024-11-25T12:12:50Z

@shubham1g5 Why did the developer bring a lint roller to the code review? Because we used SuppressLint to silence those pesky errors — nothing escapes our lint-taming prowess!

shubham1g5

@melmathari Indeed lol. Although what I have been trying to say is don't worry about the lint check and let it fail. No roller needed 🤣

app/src/org/commcare/activities/components/ImageCaptureProcessing.java

app/src/org/commcare/utils/FileUtil.java

Change requests change request SuppressLint SuppressLint imageCaptureProcessing.java

shubham1g5 · 2024-11-26T05:58:07Z

@melmathari We are seeing instrumentation test failures on this PR which looks related to media handling.

You should be able to run these tests locally to look into the failures.

melmathari · 2024-11-26T06:28:20Z

@shubham1g5 Thanks, I will take some time to test this a bit more locally

shubham1g5

Curious if the error with tests was due to us trying to copy the exif data from non image files ?

app/src/org/commcare/utils/FileUtil.java

The file isn't an image (mimeType check) The image doesn't have EXIF data (ExifInterface handles this gracefully) There are any issues reading/writing EXIF data (caught and logged) The only failure that would propagate up is if the initial file copy fails, which is appropriate since that's the core operation we need to succeed.

melmathari · 2024-11-29T08:17:53Z

Curious if the error with tests was due to us trying to copy the exif data from non image files ?

Correct, this was the case.

algora-pbc bot mentioned this pull request Nov 24, 2024

Passthrough Allow-Listed EXIF properties (GPS specifically) with resized images #2689

Open

algora-pbc bot added the 🙋 Bounty claim label Nov 24, 2024

melmathari mentioned this pull request Nov 24, 2024

Respect image orientation during resizing #2743

Open

kiran-tripathy requested review from shubham1g5, ctsims and avazirna November 25, 2024 08:05

shubham1g5 requested changes Nov 25, 2024

View reviewed changes

melmathari force-pushed the 2689 branch from 11836a1 to a9c3b5c Compare November 25, 2024 11:23

shubham1g5 requested changes Nov 25, 2024

View reviewed changes

melmathari force-pushed the 2689 branch from e9cc357 to b981ce8 Compare November 25, 2024 12:16

shubham1g5 requested changes Nov 25, 2024

View reviewed changes

app/src/org/commcare/activities/components/ImageCaptureProcessing.java Outdated Show resolved Hide resolved

app/src/org/commcare/utils/FileUtil.java Outdated Show resolved Hide resolved

app/src/org/commcare/utils/FileUtil.java Outdated Show resolved Hide resolved

melmathari force-pushed the 2689 branch 4 times, most recently from 63bee37 to e9f1985 Compare November 25, 2024 13:04

imageCaptureProcessing maintain EXIF

62ab7b7

Change requests change request SuppressLint SuppressLint imageCaptureProcessing.java

melmathari force-pushed the 2689 branch from e9f1985 to 62ab7b7 Compare November 25, 2024 13:05

melmathari added 2 commits November 25, 2024 14:07

reverted some changes

f4f9580

remove surpresslint

88e9994

Media tests

6016f4a

melmathari added 2 commits November 28, 2024 19:31

Media Test

72aea77

FileUtil changes

a15c106

shubham1g5 requested changes Nov 29, 2024

View reviewed changes

app/src/org/commcare/utils/FileUtil.java Outdated Show resolved Hide resolved

melmathari added 2 commits November 29, 2024 09:13

restore classpath and project

83b729b

Merge branch 'dimagi:master' into 2689

6ae3f3d

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

imageCaptureProcessing maintain EXIF #2902

imageCaptureProcessing maintain EXIF #2902

melmathari commented Nov 24, 2024 •

edited

Loading

damagatchi commented Nov 24, 2024

algora-pbc bot commented Nov 24, 2024

shubham1g5 left a comment

shubham1g5 Nov 25, 2024

shubham1g5 Nov 25, 2024

shubham1g5 Nov 25, 2024

shubham1g5 commented Nov 25, 2024

melmathari commented Nov 25, 2024

shubham1g5 left a comment

shubham1g5 Nov 25, 2024

shubham1g5 Nov 25, 2024

shubham1g5 Nov 25, 2024

shubham1g5 Nov 25, 2024

shubham1g5 commented Nov 25, 2024

melmathari commented Nov 25, 2024

shubham1g5 Nov 25, 2024 •

edited

Loading

shubham1g5 Nov 25, 2024

shubham1g5 Nov 25, 2024

melmathari commented Nov 25, 2024

shubham1g5 left a comment

shubham1g5 commented Nov 26, 2024

melmathari commented Nov 26, 2024

shubham1g5 left a comment

melmathari commented Nov 29, 2024

		<ignore path="src/org/commcare/utils/FileUtil.java" />
		<ignore path="src/org/commcare/activities/components/ImageCaptureProcessing.java" />

imageCaptureProcessing maintain EXIF #2902

Are you sure you want to change the base?

imageCaptureProcessing maintain EXIF #2902

Conversation

melmathari commented Nov 24, 2024 • edited Loading

The Fix

How We Pulled It Off

Test Drive

Tools to Verify

Additional Notes

damagatchi commented Nov 24, 2024

algora-pbc bot commented Nov 24, 2024

shubham1g5 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shubham1g5 commented Nov 25, 2024

melmathari commented Nov 25, 2024

shubham1g5 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shubham1g5 commented Nov 25, 2024

melmathari commented Nov 25, 2024

shubham1g5 Nov 25, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

melmathari commented Nov 25, 2024

shubham1g5 left a comment

Choose a reason for hiding this comment

shubham1g5 commented Nov 26, 2024

melmathari commented Nov 26, 2024

shubham1g5 left a comment

Choose a reason for hiding this comment

melmathari commented Nov 29, 2024

melmathari commented Nov 24, 2024 •

edited

Loading

shubham1g5 Nov 25, 2024 •

edited

Loading